Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add support for Edge transport nodes which were created externally #1473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ksamoray
Copy link
Collaborator

Normally users create Edge Transport Nodes within NSX, which deploys them into the regsitered compute manager. However, users have the option to do the same by deploying the Edge appliance anywhere, outside NSX, and registering it with NSX using the Edge CLI or OVA parameters. Later, this Edge appliance can be converted into a transport node using NSX API - this change attempts to utilize this capability.

Fixes: #1459

@ksamoray ksamoray force-pushed the edge_ova_create branch 2 times, most recently from e20b1dc to d9d4d11 Compare November 27, 2024 14:30
Optional: true,
Computed: true,
Description: "Unique Id of the fabric node",
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a bad idea, but I'll suggest it anyway.
As this resource is not generated, we do not have to fully respect the NSX API schema.

Perhaps we can move node_id in deployment_config and specify that it conflictsWith vm_deployment_config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deployment_config block contains stuff which is related to the Edge VM deployment: also attributes which are outside the vm_deployment_config block is related, e.g form_factor. Also node_user_settings is related.
And as these are required for a non-pre-deployed, we'll have to change those to optional for pre-deployed edges.
In fact I think that it's a good idea to mark deployment_config and node_id as conflicting.

fail.
So the provider will ignore node_deployment_info properties when node_id has a value - which would mean that
this edge appliance was created externally.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be helpful to also add a log statement here for debugging purposes.

e.g: "nodeID not specified, will deploy edge using values in deploymentConfig"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

var nodeDeploymentInfo *data.StructValue
if nodeID == "" {
/*
node_id attribute conflicts with node_deployment_info. As node_deployment_info is a complex object which has
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please clarify - even when specifying node_id, user still needs to populate node_deployment_info? What properties are needed?
I don't think node_deployment_info is Computed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resource has no node_deployment_info attribute as it's not implementing the entire API, as host nodes are created with policy API. Relevant node_deployment_info attributes are placed under deployment_config, and directly under the resource root.
I've set these attributes as conflicting with node_id.

@@ -725,6 +745,16 @@ func resourceNsxtEdgeTransportNodeCreate(d *schema.ResourceData, m interface{})
connector := getPolicyConnector(m)
client := nsx.NewTransportNodesClient(connector)

nodeID := d.Get("node_id").(string)
if nodeID != "" {
_, err := client.Get(nodeID)
Copy link
Collaborator

@annakhm annakhm Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to set revision/other computed properties here?

@annakhm annakhm changed the title Add support for Edge transport nodes which were created externally [WIP] Add support for Edge transport nodes which were created externally Dec 12, 2024
@salv-orlando salv-orlando added this to the v3.8.0 milestone Dec 13, 2024
@salv-orlando
Copy link
Member

Keeping this PR in 3.8.0 milestone as we're almost done with the review process.

Normally users create Edge Transport Nodes within NSX, which deploys them into the regsitered compute manager.
However, users have the option to do the same by deploying the Edge appliance anywhere, outside NSX, and registering it with NSX using the Edge CLI or OVA parameters.
Later, this Edge appliance can be converted into a transport node using NSX API - this change attempts to utilize this capability.

Fixes: vmware#1459
Signed-off-by: Kobi Samoray <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nsxt_edge_transport_node not usable for existing edge
3 participants